-
Notifications
You must be signed in to change notification settings - Fork 2
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
update kzgbn254 function to work with eigenDA encoded blobs #2
Conversation
Awesome work @afkbyte. Can we rebase and clean up comments etc? Would love to get this merged. |
arbitrator/prover/src/kzgbn254.rs
Outdated
|
||
// blob header is the first 32 bytes of the blob bytes | ||
let blob_header = blob_bytes[..32].to_vec(); | ||
println!("blob header {:?}", blob_header); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
another one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
arbitrator/prover/src/kzgbn254.rs
Outdated
|
||
temp_buffer.reverse(); | ||
buffer.extend(temp_buffer); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't to_byte_array
from the lib provide something similar or is this something more specific than that ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so it's a bit different, to_byte_array
deserializes them in big endian but I need them in little endian
arbitrator/prover/src/utils.rs
Outdated
@@ -297,26 +297,18 @@ pub fn hash_preimage(preimage: &[u8], ty: PreimageType) -> Result<[u8; 32]> { | |||
3000 | |||
).unwrap(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You might want to use Prod numbers for these. In the lib, there is mainnet-data folder under tests. You can use those files to specify the input files.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mostly stylistic concerns around using idiomatic rust - otherwise builds and arbitrator tests are failing which should ideally be passing before merging
Also looks like we're introducing cargo.lock dependency changes which are causing jit compilations to fail in CI - lets fix that too before merging https://github.com/Layr-Labs/nitro-private/actions/runs/9370515667/job/25797665060?pr=2 |
… to ignore kzg-bn254
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
works with blobs encoded by Layr-Labs/eigenda#583
also uses some modified functions from the kzg library, diff can be found here:
https://github.com/Layr-Labs/rust-kzg-bn254/compare/master...afkbyte:rust-kzg-bn254:master?expand=1
when we recieve the data from the preimage it's already in evaluation formation not coefficient (has been ifftd before being dispersed) so we don't need to do a
g1_ifft
on the g1 SRS bc kzgcommitment = <C, IFFT(SRS)> = <IFFT(C), SRS>